Introduce KeySpacePath.importData to import previously exported data #3578

ScottDugas · 2025-09-09T16:47:32Z

This introduces a new KeySpacePath.importData that will import DataInKeySpacePath as gathered by KeySpacePath.exportAllData.

The new method works when importing data exported from other clusters.

Resolves: #3573
Resolves: #3751 -- I thought I was going to pull this out, but went back and resolved it with a mapPipelined cursor.

...main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImpl.java

ohadzeliger · 2025-11-12T21:35:50Z

...main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImpl.java

+                    }
+
+                    // Store the data
+                    byte[] keyBytes = keyTuple.pack();


Should we add some fdb timer metrics for future use (imported_count)?

Yeah, I think a timer around importFuture makes sense.

...record-layer-core/src/test/java/com/apple/foundationdb/record/test/FDBDatabaseExtension.java

...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java

...main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImpl.java

...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java

ohadzeliger · 2025-11-12T23:26:14Z

...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java

+
+        verifySingleKey(dataPath, Tuple.from("item"), Tuple.from("final_value"));
+    }
+


Additional potential tests:

Large data (or any out of band error) during import

Import into partial path (no leaves in import data) + some remainders

import where data is of the wrong type

Yes, a test of more data than can be inserted into a single transaction would make sense, but not if I move it to ResolvedKeySpacePath and just have it take a single DataInKeySpacePath.

I'm not sure what you mean by a partial path.

If by data you mean the value, there is no validation, and it is not KeySpacePaths responsibility to know what is in the data. If you mean the object in the path, that should be validated above this call, and should be trust-worthy by the time you get a DataInKeySpacePath. Ideally this would be validated when you create the KeySpacePath, but it is covered in the serialization work, and I explain a bit more on the situation there: https://github.com/FoundationDB/fdb-record-layer/pull/3747/files#diff-15120b2e222e6bb7c2647b670f676b719cce8602e410487604bc87e9ea30a3b0R179

By the second bullet I meant importing into the middle of the path, as when you have a path defined for /company/employee/id/profile and the import only has /company/employee

In doing this, I had to rework the test for overwriting data, and in doing so, I decided it would be better to have 3 tests.

Now all will both run by copying back to the same cluster, and copying between clusters.

Also needed equals & hashCode & toString on DataInKeySpacePathTest so added tests for those

ohadzeliger · 2025-11-17T11:07:20Z

...er-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/FDBStoreTimer.java

        /** The amount of time checking if a {@link com.google.common.collect.RangeSet} is empty. */
        RANGE_SET_IS_EMPTY("range set is empty"),
+        /** The amount of time importing a single KeyValue into a path. */
+        IMPORT_DATA("import KeyValue"),


Is this being used?

alecgrieser

I think the only important thing is to make sure we set the executor in the fromIterator cursor, and the rest of these are just informational/minor

...src/main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePath.java

...main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImpl.java

...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java

alecgrieser · 2025-12-01T15:20:11Z

...com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImportDataTest.java

+    }
+
+    @ParameterizedTest
+    @EnumSource(CopyConfig.class)


Using the CopyConfig parameter like this is a little weird, but I can see how it's working. The alternative would, I think, be to introduce a new Extension that can select the source database and destination database for you, and then to set up the clusters appropriately in setUp so that the source matches the destination if and only if we're in CopyConfig.WithinCluster mode. I can buy that that's a bit too much machinery for this PR, if you wanted to separate that out (if you thought it was a good idea)

I'm not entirely sure I follow.
I wanted a handful of these tests to run both when copying within a cluster, and between clusters. A new extension would need to be smart enough to only apply to appropriate tests.
In my mind CopyConfig is just a slightly more readable version of @BooleanSource("withinCluster").

The thing that's weird to me is that the class sets up the clusters to be different (if there's more than one around), and then the copy config overrides that. In my mind, it would be nice if setup set those two clusters consistently.

It is possible that there would be too much refactoring involved here to make that work in a way that only the relevant tests run in both modes.

...main/java/com/apple/foundationdb/record/provider/foundationdb/keyspace/KeySpacePathImpl.java

Also, clean up some comment wording Co-authored-by: Alec Grieser <[email protected]>

ScottDugas force-pushed the keyspace-import branch from 6a071ed to abd67ac Compare October 24, 2025 18:29

ScottDugas added 11 commits November 7, 2025 16:29

Initial pass at KeySpacePath.importData

178cd61

Cleanup some of the tests for importing data

938d240

Cleanup import tests

bee1806

A little more test cleanup

44d609b

Respond to api change on main for DataInKeySpacePath

b810497

Extract helper for export+import

9132184

Change the import test to use 2 clusters if available

f63ae18

Respond to DataInKeySpacePath not having Resolved on main (after rebase)

714faf3

Cleanup some of the export tests

c255044

Fix minor typo

f30cb54

Reduce some duplication in import tests

287ff3f

ScottDugas force-pushed the keyspace-import branch from abd67ac to 287ff3f Compare November 7, 2025 21:53

ScottDugas added 2 commits November 9, 2025 15:58

Remove unused parameter

2cac9f3

Create better helpers to simplify test

77d85a2

ScottDugas added the enhancement New feature or request label Nov 9, 2025

ScottDugas changed the title ~~Keyspace import~~ Introduce KeySpacePath.importData to import previously exported data Nov 9, 2025

Minor cleanup after self-review

afc98fb

ScottDugas requested review from alecgrieser and ohadzeliger November 10, 2025 16:08

ScottDugas marked this pull request as ready for review November 10, 2025 16:08

ohadzeliger requested changes Nov 12, 2025

View reviewed changes

ScottDugas added 3 commits November 13, 2025 11:43

Instrument the time to import each data entry

7dfcb34

Change KeySpacePathImportDataTest to have separate DB fields

e6b605e

In doing this, I had to rework the test for overwriting data, and in doing so, I decided it would be better to have 3 tests.

Change tests that copyData to all be parameterized

7a4e127

Now all will both run by copying back to the same cluster, and copying between clusters.

ScottDugas mentioned this pull request Nov 13, 2025

Improve handling of importing with DirectoryLayerDirectory #3751

Closed

ScottDugas added 4 commits November 13, 2025 17:04

Test Importing a lot of data at once

40055dd

Add a test of importing a bunch of data

4c9b11e

Also needed equals & hashCode & toString on DataInKeySpacePathTest so added tests for those

Add comment to clearPath about how it doesn't clear DirectoryLayer

1a23763

Rename FDBDatabaseExtension.getDatabases -> getRandomDatabaseSubset

0fb43b9

ScottDugas added 2 commits November 14, 2025 11:02

Test less large imports with the directory layer

0ec5181

Handle creating directory layer entries better

30b9a01

ohadzeliger requested changes Nov 17, 2025

View reviewed changes

ScottDugas requested a review from ohadzeliger November 17, 2025 13:06

Add back timing IMPORT_DATA

1f78a84

ohadzeliger approved these changes Nov 17, 2025

View reviewed changes

alecgrieser requested changes Dec 1, 2025

View reviewed changes

ScottDugas and others added 2 commits December 1, 2025 15:24

Provide an executor to fromIterator

813f6e3

Also, clean up some comment wording Co-authored-by: Alec Grieser <[email protected]>

Added some comments explaining reasoning

3099acd

alecgrieser approved these changes Dec 2, 2025

View reviewed changes

ScottDugas merged commit 564d9dd into FoundationDB:main Dec 2, 2025
8 checks passed


		verifySingleKey(dataPath, Tuple.from("item"), Tuple.from("final_value"));
		}

Introduce KeySpacePath.importData to import previously exported data #3578

Introduce KeySpacePath.importData to import previously exported data #3578

Uh oh!

Conversation

ScottDugas commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alecgrieser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ScottDugas commented Sep 9, 2025 •

edited

Loading